Skip to content

getNextFileName: Get info wether filename is a file or directory #8068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

getNextFileName: Get info wether filename is a file or directory #8068

wants to merge 3 commits into from

Conversation

tueddy
Copy link
Contributor

@tueddy tueddy commented Apr 12, 2023

Hi Arduino-ESP32 team,

the FS function getNextFilename() has been merged since 2.0.6 and speeds-up directory listing by factor >20 (many files on SD-card).
Thank's for this improvement!

Unfortunately, this function can only be used to a limited extent because it is not possible to distinguish whether the returned name is a file or a directory. This may have been overlooked in PR #7229 , see discussion.
A concrete use case is an SD card web server that delivers a JSON file/directory structure for given path.

I have overloaded the function with a new return parameter isDir. This returns true if it is a directory:

    String getNextFileName(void);
    String getNextFileName(boolean *isDir);

Changes are minimal & this PR should be fully backwards compatible.

Thank's
Dirk

Directory listing in the browser with openNextFile(). Loading one directory-level takes 2s:

Slow

Directory listing in the browser with getNextFileName(boolean *isDir). Loading one directory-level takes 20ms:

Fast

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

tueddy seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@tueddy tueddy changed the title getNextFileName(boolean *isDir): get info if filename is a file or dir getNextFileName: Get info wether filename is a file or directory Apr 12, 2023
P-R-O-C-H-Y
P-R-O-C-H-Y approved these changes Apr 14, 2023
View reviewed changes
Copy link
Member

@P-R-O-C-H-Y P-R-O-C-H-Y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks for contribution @tueddy :)
Please submit the CLA to be able to merge this.

@P-R-O-C-H-Y P-R-O-C-H-Y self-assigned this Apr 14, 2023
@tueddy
Copy link
Contributor Author

tueddy commented Apr 14, 2023

@P-R-O-C-H-Y Thank's for approving!
I had problems signing the CLA beacause my email was missing in first commit.
I have commited a small enhancement, checking for isDir != NULL to avoid a crash.
With this commit i hope the CLA signing was successful now..

P-R-O-C-H-Y
P-R-O-C-H-Y requested changes Apr 14, 2023
View reviewed changes
libraries/FS/src/vfs_api.cpp Outdated
Comment on lines 561 to 564
// check entry is a directory
if(isDir) {
*isDir = (file->d_type == DT_DIR);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That check is good for sure. Can you just remove the spaces before the part so its aligned with other code?

@P-R-O-C-H-Y
Copy link
Member

@tueddy Seems that CLA is still not signed yet.

@P-R-O-C-H-Y P-R-O-C-H-Y added the Area: Libraries Issue is related to Library support. label Apr 14, 2023
@P-R-O-C-H-Y P-R-O-C-H-Y added this to the 2.0.8 milestone Apr 14, 2023
@tueddy
Copy link
Contributor Author

tueddy commented Apr 14, 2023

Don't know what's wrong with signing:

grafik

@P-R-O-C-H-Y
Copy link
Member

Seems that the first commit doesn't know your GITHUB, and that's why this is happening..

Snímek obrazovky 2023-04-14 v 11 02 46

@fschrempf
Copy link

Don't know what's wrong with signing:

I suspect it's because the first of your three commits still has the wrong author/e-mail. You should probably squash the commits into one and fix the authorship.

@P-R-O-C-H-Y
Copy link
Member

Don't know what's wrong with signing:

I suspect it's because the first of your three commits still has the wrong author/e-mail. You should probably squash the commits into one and fix the authorship.

That should help. You can do squash yourself on your branch so you will still be the author :)

@P-R-O-C-H-Y
Copy link
Member

@tueddy If there is still a problem, it will be faster to close this PR, create new branch and copy the changes there. After that open new PR and ping me to comment and I will approve it ASAP :)

tueddy added a commit to tueddy/arduino-esp32 that referenced this pull request Apr 14, 2023
@tueddy
getNextFileName: Get info whether filename is a file or directory
fe5eb98 
re-submit espressif#8068
@tueddy
Copy link
Contributor Author

tueddy commented Apr 14, 2023

I will close here & open a new PR..

@tueddy tueddy closed this Apr 14, 2023
@tueddy tueddy mentioned this pull request Apr 14, 2023
Merged
4 tasks
me-no-dev pushed a commit that referenced this pull request Apr 17, 2023
@tueddy
getNextFileName: Get info whether filename is a file or directory (#8079
fa9e059 
)

re-submit #8068
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@P-R-O-C-H-Y P-R-O-C-H-Y P-R-O-C-H-Y approved these changes

Assignees

@P-R-O-C-H-Y P-R-O-C-H-Y

Labels
Area: Libraries Issue is related to Library support.
Projects
None yet
Milestone
2.0.8
Development

Successfully merging this pull request may close these issues.
4 participants
@tueddy @CLAassistant @P-R-O-C-H-Y @fschrempf